[v3] feat: a fixed directory layout for the test workspace - #49
Conversation
Specs live in tests/, runner plugins in plugins/, and everything an
environment writes at run time goes to generated/<environment>/ — so a
local server lands under the workspace instead of a run/ directory next
to build.gradle.kts, and two local environments in one matrix stop
sharing a server directory.
A workspace still holding its specs at the root is moved into tests/ the
first time plugwrightCompileTests runs, tsconfig.json included: its
include still points at where the specs used to be.
plugins { local("stand-reset") } names a plugin instead of spelling out
the path its compiled form ends up at.
plugwrightInit now creates tests/example.spec.ts, plugins/example-plugin.ts and a .gitignore covering node_modules, dist and generated. An existing .gitignore gets the missing lines appended rather than replaced. The scaffolded files live in resources as real .ts/.json files instead of Kotlin string literals, so an editor checks them and nothing needs escaping. The runner dependency follows the plugin's own version instead of a range last updated by hand.
Specs go to tests/, the server the local environment starts goes to generated/local/run, and the stand-reset plugin is named rather than pointed at through dist/. npm cannot install a package linked by path while that package has a prepare script: it packs the directory into a staging copy without dev dependencies, so the build there fails and takes the whole install with it. The three local packages are built up front instead, by the same two scripts CI now runs.
Adds a page describing the directories — tests, plugins, dist, generated — and what happens to a project still laid out the old way. The pages that showed runDir, a path to a compiled plugin, or a spec at the root of testsDir now show the current shape instead.
Landed at the workspace root in c4a89a4, after this branch moved the rest of the suite into tests/. The migration only runs while there is no tests/ directory, so nothing would have picked it up and the spec would have stopped running.
The linked plugin packages moved to 3.0.0-dev.0 and their peer range to >=3.0.0-dev.0 in the previous PR; the example's lockfile still recorded 1.0.0 and >=2.0.0. Written by 'npm install', no dependency changed.
Drownek
left a comment
There was a problem hiding this comment.
The PR looks very solid overall! The shift to a more structured directory layout (tests/, plugins/, dist/, generated/) makes the workspace much cleaner and better separates user code from build artifacts and server runtimes. The migration logic and the plugwright-workspace: URI scheme for local("plugin-name") are great additions.
I did spot one unintended bug regarding the tsconfig.json migration:
Gson isLenient silently strips tsconfig.json comments
In PlugwrightCompileTestsTask.kt, retargetTsConfig parses the tsconfig.json using Gson. The PR description notes that if the file has comments, it should be left untouched. However, by adding .apply { isLenient = true } to the JsonReader, Gson will successfully parse the file (ignoring the comments). Since Gson's AST (JsonObject) does not store comments, when the file is rewritten via GsonBuilder().toJson(config), all original comments are permanently deleted.
If the intention was to deliberately fail parsing so the catch block leaves the commented file untouched, you should remove isLenient = true (which will make it fail on comments, but also on trailing commas), or manually check for comments in the file string before passing it to Gson.
Otherwise, LGTM!
Gson's isLenient=true made retargetTsConfig successfully parse a tsconfig.json with comments, then rewrite it as plain JSON — deleting the comments the try/catch was meant to leave untouched. Drop isLenient so a commented config fails to parse and falls through to the existing catch, which warns and leaves the file alone. Spotted by Drownek in PR Drownek#49 review.
|
Good catch — fixed. Dropped While I was in there I swapped Checked both paths: a Commit: 6f9726c |
Third in the series (#46), on top of #48. Thanks for merging that one as a merge commit again — this
branch shows only its own 6 commits.
Rebased onto
v3-devat67e78ce. No conflicts, but the rebase did turn up two things thatneeded a commit of their own; they are at the end of this description. Kotlin compiles, and
tsc --noEmitis clean in the example workspace.Specs, compiled output and whatever a server writes while it runs all used to land wherever the
build script pointed them. With more than one environment running at once, that stopped being
workable — two
localenvironments in the same build would have shared onerun/directory.The layout
Everything sits under
testsDir(src/test/e2eby default):Three of those are disposable —
node_modules,dist,generated. Delete any of them and thenext
plugwrightTestrecreates it.PlugwrightLayoutinplugwright-apiis the whole of it: an interface over one directory, withthe names as constants. A mode reaches it through
TaskRegistrationContext.layout, and gets achance to seed its own spec from it in the new
PlugwrightMode.applyLayoutDefaults, which runsbefore validation.
LocalModeuses that to place its server in the environment's own generateddirectory, so a build script that never mentions
runDirstill gets one and two environmentsnever collide.
local("name")for a plugin in the workspaceplugins { local("stand-reset") // src/test/e2e/plugins/stand-reset.ts }local(file(...))still works and still means that exact path. The named form goes into theconfig as a
plugwright-workspace:specifier and is resolved againstdist/pluginsbefore theconfig is written, so the runner keeps seeing a plain path.
Migrating a workspace that already exists
The compile task moves stray
*.spec.ts/*.spec.jsintotests/, and only while there is notests/directory at all — so it happens once, on the first run after the upgrade, and nevertouches a workspace that already follows the layout. It reports what it moved.
The
tsconfig.jsonis retargeted with it:includebecomestests/**/*.tsandplugins/**/*.ts,outDirbecomes./dist. A config the JSON parser chokes on is left alonewith a message saying what to change by hand — comments are legal in a
tsconfig.json, andrewriting one that has them would drop them.
One behaviour change worth flagging
runDirno longer defaults to<project>/run. It defaults to nothing, and an unset value iswhat tells the local mode to place the server under
<testsDir>/generated/<environment>/run.For a 2.x build script that never set
runDir, the first run after upgrading starts a server ina new empty directory rather than the existing
run/— a fresh world, and no plugin configcarried over. The old directory is left alone, so nothing is lost, but it is also not picked up.
One line puts it back:
environments { create("local", LocalMode) { runDir.set(layout.projectDirectory.dir("run")) } }I did not add an automatic move for this. A server directory can be large, it can be running, and
guessing which of several environments should inherit it is not something the build can do
safely.
plugwrightInitIt now writes the layout:
tests/example.spec.ts,plugins/example-plugin.ts,package.json,tsconfig.json, and a.gitignorecoveringnode_modules,distandgenerated. Each file iswritten only when it is absent. An existing
.gitignoregets the missing lines appended ratherthan being replaced — the workspace may well have entries of its own.
The scaffolded files moved out of the Kotlin source into
plugwright-core/src/main/resources/plugwright-init, so they are real.tsand.jsonfiles aneditor can check, with nothing escaped past the Kotlin parser. The one placeholder is
@runnerVersion@, filled from the plugin's own version down to the minor —3.0.1scaffolds"@drownek/plugwright": "^3.0.0". That also fixes the stale^2.0.3the old template hardcoded,which would have scaffolded a 2.x runner into a 3.0 workspace.
The example plugin it writes is a runner plugin, not a Paper one — a
definePluginwith abeforeEachand a fixture, plus thedeclare moduleblock that types the fixture. It is notloaded until you say so; the task prints the
plugins { local("example-plugin") }line to add.Two fixes that ride along
ci.ymlbuilt onlyrunner-package, so the example's two linked plugin packages went into therun unbuilt. It now runs
npm run install:packages/npm run build:packages, two new scripts inthe root
package.jsonthat cover all three.runner-package'spreparescript is gone. npm runspreparewhen a package is installed from adirectory, and it packs that directory into a staging copy that never gets its dev dependencies —
so
file:linking the runner failed onrimraf: not foundbefore it could reachtsc.prepublishOnlystill builds before a publish, which is the case that mattered.What the rebase turned up
message-buffer.spec.tscame in with yourc4a89a4, at the workspace root, after this branch hadalready moved the rest of the suite into
tests/. The migration only runs while there is notests/directory, so nothing would have picked it up and the spec would have quietly stoppedrunning. It is moved in its own commit.
The example's lockfile still recorded the two plugin packages at
1.0.0with a>=2.0.0peerrange, from before #48 moved them to
3.0.0-dev.0. Refreshed withnpm install; no dependencychanged.
On the npm scope
This branch still says
@drownek/plugwrighteverywhere, including in the init template. That isdeliberate — it matches what
v3-devpublishes today, and the rename to@plugwright/runnerisin PR 6 where you said it fits. The template takes it as one string, so the rename touches one
line here.
Docs
docs/project-layout.mdxis new.configuration.mdx,quickstart.mdx,custom-modes.mdx,plugins.mdx,environments.mdx,writing-tests.mdxand the README are updated for the layoutand for
local("name").Merging
Same request: merge commit or rebase and merge, not squash and merge — PRs 4 through 6
are stacked on this branch. I'll rebase PR 4 onto
v3-devand open it once this lands.